Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add generatorLibrary options and allow faker to select #93

Merged
merged 12 commits into from
Sep 29, 2022

Conversation

MH4GF
Copy link
Contributor

@MH4GF MH4GF commented Sep 27, 2022

closes #90, related #92

What

Add a new option generateLibrary to make faker available in addition to the existing casual.

Context

casual dependents on the node API(e.g. fs) and cannot be made to work in the browser.
ref: #90

Additional Information

To simplify the code, we also added a class that returns the mock value generated by the library.

@MH4GF
Copy link
Contributor Author

MH4GF commented Sep 27, 2022

✅ TODO

  • refactor to add CasualMockValueGenerator class
  • Eliminate "casual" import from index.ts
  • add FakerMockValueGenerator class
  • add generateLibrary option and switching MockValueGenerator
  • add description of generateLibrary option to README

@MH4GF MH4GF force-pushed the add-generator-library-option branch from eec74c1 to 4c19722 Compare September 27, 2022 01:36
Copy link
Owner

@ardeois ardeois left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, it's going in the right direction 🎉

Because faker almost completed migrating to TypeScript so that DefinitelyTyped no longer needs to maintain its external [@types/faker](https://www.npmjs.com/package/@types/faker) package. ref: https://fakerjs.dev/about/announcements/2022-01-14.html
As shown in the following code, the result changes even if the seed is added:
```
> const { faker } = require("@faker-js/faker")
undefined
> faker.seed(0)
0
> const a = faker.date.past().toISOString()
undefined
> a
'2022-03-08T21:45:15.158Z'
> faker.seed(0)
0
> const b = faker.date.past().toISOString()
undefined
> b
'2022-03-08T21:45:30.411Z'
> a === b
false
```

This is because the default refDate passed in the second argument of faker.date.past() is now, so we can fix it at a specific date.

ref: faker-js/faker#859
@@ -36,7 +37,6 @@
"devDependencies": {
"@auto-it/conventional-commits": "^10.33.0",
"@graphql-codegen/testing": "^1.17.7",
"@types/faker": "^5.5.9",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was probably left in there because it was forgotten to erase it, but now the type definitions have been imported into the repository and are no longer needed.
5a8cd1b

? `faker.date.past().toISOString(1, new Date(2022, 0))`
: `'${faker.date.past(1, new Date(2022, 0)).toISOString()}'`;
seed = (seed: number) => faker.seed(seed);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compatibility of the execution results of each function is checked here:
#92 (comment)

@MH4GF MH4GF marked this pull request as ready for review September 28, 2022 14:56
Copy link
Owner

@ardeois ardeois left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work @MH4GF thank you!

I think you cover all use cases. I'll run the CI and will try the package locally before merging
I'll do that before tomorrow

tests/generateLibrary/faker/spec.ts Show resolved Hide resolved
@ardeois ardeois self-assigned this Sep 28, 2022
@ardeois ardeois added the minor Increment the minor version when merged label Sep 29, 2022
@ardeois
Copy link
Owner

ardeois commented Sep 29, 2022

@MH4GF all good, merging and closing the issue

@ardeois ardeois merged commit 1f8c00b into ardeois:main Sep 29, 2022
@MH4GF MH4GF deleted the add-generator-library-option branch September 30, 2022 01:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Increment the minor version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dynamicValues option does not work in browser
2 participants